Implementation of `sync_nonpoison` and `nonpoison_mutex` #134663

Aandreba · 2024-12-22T19:47:26Z

Implementation of sync_nonpoison and nonpoison_mutex
Docs come straight from the poisoned Mutex, so they need to be updated.

Tracked by:

Tracking Issue for sync_nonpoison and nonpoison_{condvar,mutex,once,rwlock} #134645

rustbot · 2024-12-22T19:47:35Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @thomcc (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

library/std/src/sync/nonpoison/mutex.rs

tgross35 · 2024-12-23T21:55:37Z

@Amanieu you may want to double check this

Noratrieb · 2024-12-24T12:03:02Z

Looks like you messed up a rebase. No worries, mistakes like this happen sadly (thanks Git for being so hard to use!). To get rid of all these commits, do a git rebase -i upstream/master (where upstream is the rust-lang/rust remote) as described in https://rustc-dev-guide.rust-lang.org/git.html#i-made-a-merge-commit-by-accident.

Amanieu · 2024-12-24T15:50:42Z

This seems fine as a first step. Longer term we may want to consider changing the poisoning mutexes to be a wrapper around a non-poisoning Mutex<Poison<T>>, but that can happen in a later PR.

Co-authored-by: oxalica <[email protected]>

Aandreba · 2025-06-12T14:40:41Z

Mind rebasing at least? If the tests are still not blessing for you, I'll see if I can push a commit that updates them.

Done! Have a try, let me know If you need anything!

tgross35 · 2025-06-12T18:31:34Z

So, UI tests have two things that get checked:

Error annotations
Stderr

Error annotations just highlight the important changes / messages you want to see and need to get updated manually. The stderr files show what the full error output looks like, and get updated with --bless. I added a commit that did both, in most cases it just meant adding std::sync to the error annotations.

Take a read through https://rustc-dev-guide.rust-lang.org/tests/ui.html, it explains this in better detail.

rust-log-analyzer · 2025-06-12T18:35:16Z

The job mingw-check-tidy failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

##[endgroup]
Executing TIDY_PRINT_DIFF=1 npm install eslint@$(head -n 1 /tmp/eslint.version) &&  python2.7 ../x.py test --stage 0 src/tools/tidy tidyselftest --extra-checks=py,cpp
+ head -n 1 /tmp/eslint.version
+ TIDY_PRINT_DIFF=1 npm install [email protected]
npm ERR! code E504
npm ERR! 504 Gateway Timeout - GET https://registry.npmjs.org/@humanwhocodes%2fconfig-array

npm ERR! A complete log of this run can be found in: /home/user/.npm/_logs/2025-06-12T18_33_10_490Z-debug-0.log
  local time: Thu Jun 12 18:35:07 UTC 2025
  network time: Thu, 12 Jun 2025 18:35:07 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

tgross35

A few more small comments. Note that there are still some small unresolved comments above that need to be addressed.

Ignore the tidy check failure, looks like NPM might be partially down.

tgross35 · 2025-06-12T18:33:00Z

library/std/src/sync/nonpoison/mod.rs

@@ -0,0 +1,10 @@
+//! Non-poisoning syncronous locks.


s/syncronous/synchronous/

tgross35 · 2025-06-12T18:34:44Z

library/std/src/sync/nonpoison/mutex.rs

+/// A mutual exclusion primitive useful for protecting shared data.
+///
+/// For more information about mutexes, check out the documentation for the
+/// poisoning variant of this lock found at [std::sync::poison::Mutex](crate::sync::poison::Mutex).


Nit: make this [`poison::Mutex`], and then add [`poison::Mutex`]: crate::sync::poison::Mutex to the bottom of the docs section.

tgross35 · 2025-06-12T18:39:38Z

library/std/src/sync/nonpoison/mutex/tests.rs

Could you add a test to check the panic behavior?

tgross35 · 2025-06-12T18:41:46Z

library/std/src/sync/nonpoison/mutex.rs

+    #[unstable(feature = "nonpoison_mutex", issue = "134645")]
+    pub fn map<U, F>(orig: Self, f: F) -> MappedMutexGuard<'a, U>


Add a comment // #[unstable(feature = "mapped_lock_guards", issue = "117108")] so we remember this is also unstable under that gate. It would be good to add the same to any API that uses MappedMutexGuard, and the MappedMutexGuard itself.

tgross35 · 2025-07-07T11:08:10Z

@Aandreba gentle ping, are you able to get this over the line? The only things left are stable->unstable gate changes and documentation fixes, see the non-resolved review comments starting at #134663 (comment).

Let us know if you need any help.

Aandreba · 2025-07-07T23:31:56Z

@Aandreba gentle ping, are you able to get this over the line? The only things left are stable->unstable gate changes and documentation fixes, see the non-resolved review comments starting at #134663 (comment).

Let us know if you need any help.

I'm currently having problems rebasing master, so I'm stuck. Tomorrow I'll look into it more.

tgross35 · 2025-07-07T23:39:11Z

If you're getting conflicts, you should try squashing before actually doing the rebase. Something like:

# set FETCH_HEAD to current latest master
git fetch https://github.com/rust-lang/rust.git master

# Squash without changing the base with --keep-base
# In the editor, change the "pick" in every line except the first to "fixup" or "f"
# This should complete without errors
git rebase -i FETCH_HEAD --keep-base

# Now do the actual rebase onto the latest master
git rebase -i FETCH_HEAD

That way you don't have to resolve conflicts for intermediate commits that aren't relevant anymore.

Aandreba · 2025-07-14T09:44:24Z

@tgross35 The last step still gives me an error

tgross35 · 2025-07-14T17:40:42Z

Please be more specific if you need help. I tested it locally and it succeeded without conflicts.

connortsui20 · 2025-07-15T08:48:44Z

Hi @Aandreba, if your bandwidth is low, I would love to have a go at getting this to the finish line! I have some free time on my hands for the next few weeks.

Aandreba · 2025-07-16T08:39:10Z

Hi @Aandreba, if your bandwidth is low, I would love to have a go at getting this to the finish line! I have some free time on my hands for the next few weeks.

Yeah, I don't really have much time for this right now, so that would be helpful :)

Commits squashed from rust-lang#134663

connortsui20 · 2025-07-21T11:48:15Z

Whoops, sorry for the pings, I forgot I had the PR link in that first commit 🫤. Though in other news I think we will be able to get that PR through soon 🤞

Commits squashed from rust-lang#134663

rustbot assigned thomcc Dec 22, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 22, 2024

This comment has been minimized.

Sign in to view

tgross35 reviewed Dec 23, 2024

View reviewed changes

library/std/src/sync/nonpoison/mutex.rs Outdated Show resolved Hide resolved

tgross35 reviewed Dec 23, 2024

View reviewed changes

library/std/src/sync/nonpoison/mutex.rs Outdated Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

This comment has been minimized.

Sign in to view

Aandreba force-pushed the sync_nonpoison branch from b0a1973 to f023e42 Compare December 24, 2024 15:16

This comment has been minimized.

Sign in to view

Aandreba and others added 7 commits June 12, 2025 16:33

Update library/std/src/sync/nonpoison/mutex.rs

01db90f

Co-authored-by: oxalica <[email protected]>

Update library/std/src/sync/nonpoison/mutex.rs

87474c0

Co-authored-by: oxalica <[email protected]>

added non-poison mutex

66bfbbd

added nonpoison_mutex to lib enabled features

02c2cdb

fix position of enabling feature

1a1fb7c

added feature flag to doctests

466f80e

fixed incorrect doc link

eccc9fa

Aandreba force-pushed the sync_nonpoison branch from 93ca6a7 to eccc9fa Compare June 12, 2025 14:39

This comment has been minimized.

Sign in to view

Update UI tests

ac2ca14

tgross35 requested changes Jun 12, 2025

View reviewed changes

tgross35 mentioned this pull request Jun 27, 2025

Update poison.rs to fix the typo (sys->sync) #143086

Merged

connortsui20 mentioned this pull request Jul 16, 2025

Implementation: #[feature(sync_nonpoison)], #[feature(nonpoison_mutex)] #144022

Open

connortsui20 pushed a commit to connortsui20/rust that referenced this pull request Jul 16, 2025

add nonpoison module and nonpoison mutex

fe63b90

Commits squashed from rust-lang#134663

connortsui20 pushed a commit to connortsui20/rust that referenced this pull request Jul 17, 2025

add nonpoison module and nonpoison mutex

8c5aad4

Commits squashed from rust-lang#134663

connortsui20 pushed a commit to connortsui20/rust that referenced this pull request Jul 18, 2025

add nonpoison module and nonpoison mutex

34fc7a9

Commits squashed from rust-lang#134663

connortsui20 pushed a commit to connortsui20/rust that referenced this pull request Jul 20, 2025

add nonpoison module and nonpoison mutex

4fbc2f5

Commits squashed from rust-lang#134663

connortsui20 pushed a commit to connortsui20/rust that referenced this pull request Jul 21, 2025

add nonpoison module and nonpoison mutex

a9e0094

Commits squashed from rust-lang#134663

connortsui20 pushed a commit to connortsui20/rust that referenced this pull request Jul 21, 2025

add nonpoison module and nonpoison mutex

2aed473

Commits squashed from rust-lang#134663

		#[unstable(feature = "nonpoison_mutex", issue = "134645")]
		pub fn map<U, F>(orig: Self, f: F) -> MappedMutexGuard<'a, U>

Implementation of sync_nonpoison and nonpoison_mutex #134663

Are you sure you want to change the base?

Implementation of sync_nonpoison and nonpoison_mutex #134663

Conversation

Aandreba commented Dec 22, 2024

Uh oh!

rustbot commented Dec 22, 2024

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

tgross35 commented Dec 23, 2024

Uh oh!

This comment was marked as resolved.

Noratrieb commented Dec 24, 2024

Uh oh!

This comment has been minimized.

This comment has been minimized.

Amanieu commented Dec 24, 2024

Uh oh!

Aandreba commented Jun 12, 2025

Uh oh!

This comment has been minimized.

tgross35 commented Jun 12, 2025

Uh oh!

rust-log-analyzer commented Jun 12, 2025

Uh oh!

tgross35 left a comment

Choose a reason for hiding this comment

Uh oh!

tgross35 Jun 12, 2025

Choose a reason for hiding this comment

Uh oh!

tgross35 Jun 12, 2025

Choose a reason for hiding this comment

Uh oh!

tgross35 Jun 12, 2025

Choose a reason for hiding this comment

Uh oh!

tgross35 Jun 12, 2025

Choose a reason for hiding this comment

Uh oh!

tgross35 commented Jul 7, 2025

Uh oh!

Aandreba commented Jul 7, 2025

Uh oh!

tgross35 commented Jul 7, 2025

Uh oh!

Aandreba commented Jul 14, 2025

Uh oh!

tgross35 commented Jul 14, 2025

Uh oh!

connortsui20 commented Jul 15, 2025

Uh oh!

Aandreba commented Jul 16, 2025

Uh oh!

connortsui20 commented Jul 21, 2025

Uh oh!

Uh oh!

Implementation of `sync_nonpoison` and `nonpoison_mutex` #134663

Implementation of `sync_nonpoison` and `nonpoison_mutex` #134663